SearchActivity에서 DrawActivity로 넘어갈 때 1)장소 검색, 2)현위치, 3)사용자 지정 중 어떤 걸로 넘어오는지 알아야 하는데 이를 구분하기 위한 목적으로 Data Class에 mode라는 attribute를 만들었습니다.

이후 DrawActivity에서 getIntent로 값을 받을 때 mode의 값으로 가령 "currentLocation"이 들어있으면 분기 처리 중 currentLocation에 대응되는 걸 활성화시키는 식으로 코드를 짰습니다.

이 방법에 대해서 어떻게 생각하시는지 궁금합니다. 혹시 Data Class에 저렇게 식별 목적의 attribute를 추가 안 하고 더 깔끔하게 처리할 수 있는 방법이 있을까요?

override fun selectItem(item: SearchResultEntity) {
      startActivity(
          Intent(this, DrawActivity::class.java).apply {
              addFlags(Intent.FLAG_ACTIVITY_NO_ANIMATION)
              putExtra(EXTRA_SEARCH_RESULT, item) //mode == "searchLocation"
          }
      )
  }

위의 코드를 보니까 SearchResultEntity 데이터 클래스의 fullAddress, name, locationLatLng 속성은 검색 결과 리스트에서 아이템을 선택했을 때 초기화 되는 것으로 보여요!

따라서, mode 정보는 이와 별개로 초기화를 하는 것이 더 좋을 거 같아요! 현재도 mode를 제외하고 다른 속성들은 다 비어있기 때문에..!

그리고 앞으로의 확장성을 좀 더 고려하여, mode의 데이터 타입은 문자열 대신에 enum class를 활용해보는 건 어떨까요?? 오타의 발생 가능성도 줄이고, 가독성도 조금 더 올라갈 거 같습니다!

private fun initMode() {
    when (searchResult.mode) {
        "searchLocation" -> initSearchLocationMode()
        "currentLocation" -> initCurrentLocationMode()
        "customLocation" -> initCustomLocationMode()
    }
}

SearchResultEntity는 원래 장소 검색 api에서 받아온 결과값을 관리하는 목적의 data class라 저도 mode라는 attribute가 여기에 껴있는 게 어색하다고 생각하고 따로 다루는 것에 동의합니다!

생각해보니 SearchResultEntity에서 mode를 빼고 intent에 SearchResultEntity와 별도로 put을 하거나 bundle을 활용하는 방법도 있겠네요. 고민을 좀 더 해보겠습니다.

enum class 활용 좋네요. 반영하겠습니다!

현위치나 사용자 지정 출발지 설정은 fullAddress 같은 data를 DrawActivity에서 reverseGeoCoding으로 직접 생성하기 때문에 여기 SearchActivity에서는 Intent로 넣어줄 값이 없습니다. 그래서 빈문자열로 냅뒀는데 fullAddress, name의 type을 nullable로 바꾸고 null을 넣는 게 나을지, 아니면 지금처럼 non-nullable 타입으로 두고 빈문자열을 넣을지 고민이 됩니다. 어떤 게 더 나을까요?

위에 적은 리뷰 내용이 참고가 될 거 같은데요! SearchActivity에서는 mode 정보만 인텐트로 넘기고, 그 외의 속성들은 아예 넘기지 않는 것이 좋을 거 같습니다!

pr point 1번이 이 부분입니다! drawCourse에서 departureLatLng 값을 필요로 하는데 (현재 짜놓은 logic에서) 이 값에 대한 초기화는 setLocationChangedListener()에서 이루어집니다. setLocationChangedListener()는 gps에서 사용자의 현위치가 변경됐다고 인지될 때마다 호출되는 콜백함수입니다.

여기서 순서를 맞춰줘야 하는 게 어떤 거냐면 gps로 사용자의 위치를 받아오기까지 약간의 loading 시간이 필요합니다. 이 loading이 끝나서 setLocationChangedListener()가 돌아야 departureLatLng이 초기화될 수 있는데 이 loading 시간 전에 drawCourse()가 실행되니까 departureLatLng이 초기화가 안 돼서 NPE가 뜹니다.

그래서 지금 인위적으로 0.5초의 delay를 줘서 0.5초 후에 drawCourse()가 실행되게 해주었습니다. (loading 시간을 기다리기까지 여유 시간을 벌어준 것)

그런데 네트워크나 gps 상황에 따라 0.5초 이상이 걸릴 수도 있는데 이 경우엔 앱이 죽어버리겠죠. 그래서 코루틴으로 순서를 맞춰줘야겠다고 생각했는데 initMode()에서 drawCourse()부분만 따로 내어낸다고 했을 때 이것을 setLocationChangedListener()가 실행된 이후 시점에 최초 1회만 실행되도록 해주려면 어떻게 해볼 수 있을까요?

간단하게는 setLocationChangedListener() 스코프 안에서 setDepartureLatLng() 밑에 drawCourse()를 써주면 되는데 이렇게 하면 현위치가 바뀔 때마다 drawCourse()가 실행되는 것이니 리소스가 낭비됩니다. 그래서 순서를 맞춰주면서도 최초 1회만 실행이 되게 해야 합니다.

이 링크의 WineyFeedFragment 에서 withContext로 동기 처리한 코드 참고하면 도움이 될 거 같습니다!

네트워크 통신이 원활하지 않은 곳에서 GPS 가 늦게 잡히는 것은 어쩔 수 없기 때문에, 어떤 모드에서든지 동기 처리가 필수적일 거 같네요! (drawCourse 함수에서 departureLatLng가 null이 되지 않도록 -> 현위치가 정확히 잡히고 나서 코스 그리기를 시작할 수 있도록)

참고 링크

• https://todaycode.tistory.com/23
• https://kotlinworld.com/151

viewLifeCycleScope.launch {
    Timber.d("ITEM SIZE: ${wineyFeedAdapter.itemCount}")
    val currentItemSize = wineyFeedAdapter.itemCount

    if (checkRestoreScrollCondition(currentItemSize)) {
        withContext(Dispatchers.Main) {
            // 이 함수의 작업이 끝날 때까지, viewLifeCycleScope 코루틴 블록은 일시 정지 
            restoreScrollPosition()
        }
    }

    // withContext 작업이 끝나면, 여기 실행 
    dismissLoadingDialog()
    binding.rvWineyfeedPost.isVisible = wineyFeedAdapter.itemCount > 0
}

latEntr 속성이 무엇인지 알 수 있을까요?

주소의 입구에 해당하는 위도 좌표입니다.

다른 attribute에 대한 설명은 아래 링크에 있습니다!
https://openapi.sk.com/products/detail?linkMenuSeq=23

이 문서 를 통해 각 속성이 어떤 것인지 알 수 있었어요!

응답값 response 가 널이어도 LocationData에는 "buildingName fail" 같은 문자열이 들어가네요!
이 함수를 뷰모델에서 사용하고 있는데, "buildingName fail" 과 같은 상황에 대한 별도의 예외 처리는 없는 것으로 보였습니다..! 혹시 모를 예외 상황을 위해, 널에 대한 처리를 좀 더 분명하게 하면 좋을 거 같습니다 :)

interface ReverseGeocodingRepository {
    suspend fun getLocationInfoUsingLatLng(lat: Double, lon: Double): LocationData?
}

override suspend fun getLocationInfoUsingLatLng(
        lat: Double,
        lon: Double
    ): LocationData? {
        val response = reverseGeocodingDataSource
            .getLocationInfoUsingLatLng(lat = lat, lon = lon)
            .body()

        return if (response != null) {
            LocationData(
                buildingName = response.addressInfo.buildingName,
                fullAddress = response.addressInfo.fullAddress
            )
        } else null
    }

    fun getLocationInfoUsingLatLng(lat: Double, lon: Double) {
        viewModelScope.launch {
            runCatching {
                reverseGeocodingRepository.getLocationInfoUsingLatLng(lat = lat, lon = lon)
            }.onSuccess { response ->
                if(response == null) {
                    Timber.e("getLocationInfoUsingLatLng response is null")
                    return@launch
                }

                Timber.tag(ContentValues.TAG).d("통신success")
                reverseGeocodingResult.value = response
            }.onFailure {
                Timber.tag(ContentValues.TAG).d("통신failure : ${it}")
                errorMessage.value = it.message
            }
        }
    }

이런 식으로 널 처리하는 걸 생각해봤는데, 더 나은 방식이 있다면 적극 추천해주세요!!

response 자체가 null이 오는 건 생각 못했는데 좋다!! 날카로워서 베일 뻔..

내가 짜놓은 코드는 하은이 네 말처럼 response부터 null이 오는 경우도 "buildingName fail"을 주기 때문에 적절한 null 처리라는 생각이 안 드네.

api에서 buildingName, fullAddress 값으로 보내줄 게 없을 때 빈문자열("")로 보내주거든. 그래서 (api 측에 문제가 생기지 않는 이상) 사실 buildingName에 null이 뜰 가능성은 없어서 하은이 네가 짜준 코드처럼 그냥 크게 response의 null 여부만 체크하는 게 간단할 것 같아!! (만약 response는 null이 아닌데 이후 attribute 값들이 null을 가진다면 더 세부적인 처리가 필요하겠지만)

문자열은 하드코딩 하지 않고 const 로 상수화 시키는 것이 어떨까요??

아래 코드에 리뷰를 추가했는데, 문자열 대신 enum class를 활용하는 것도 좋을 거 같아요 :)

좋아요!! 매직 리터럴을 지양하는 방향이 좋다고 생각하고 기존의 코드들도 리팩토링 해오고 있었습니다!

Runnect, Tmap 이라는 한정자는 qualifier 라는 패키지로 별도 분리하여 정의하는 것이 더 좋을 거 같습니다 :)
그러면 앞에 일일이 RetrofitModule 클래스명을 적어주지 않아도 되니까요!

그렇게 해본 적이 없어서 어떻게 해볼 수 있을지 모르겠는데 코어 타임 때 여쭤보겠습니다!

getMapAsync : 비동기로 NaverMap 객체를 얻어옵니다. NaverMap 객체가 준비되면 callback의 OnMapReadyCallback.onMapReady(NaverMap)가 호출됩니다.

네 맞습니다.

그래서 지도 객체를 활용하는 logic들은 최대한 onMapReady 구현부 안에 넣어주려고 했습니다. 맵 객체가 초기화 안 됐는데 참조하려들면 NPE가 뜨니까 순서를 맞춰줘야 하는데 DrawActivity 내에 딸려있는 기능이 많다보니 이 작업이 까다로워서요.

앗 리뷰라기보다는 개인 메모로 적어뒀는데 친절한 설명 감사합니다 🙇‍♀️🙇‍♀️

함수들 간에 간격(공백)이 있으면 좋을 거 같아요 :)
이미 알고 있겠지만, Ctrl + Alt + L 이용해서 코드 자동 정렬 할 수 있습니다!

Ctrl+Alt+L 했을 때 저렇게 정렬이 되고 있는데 어떤 간격(공백)을 말씀하시는 걸까요?! (이해를 못해서 물어보는 것)

예시 들어주시면 감사하겠습니다!

앗 저렇게 보니까 무슨 얘긴지 모를 수 있겠네요! DrawActivity 클래스 안의 멤버 함수들 간에 공백 라인이 없는 경우가 많더라구요! 한 함수 안에서는 저렇게 라인이 붙어있는 게 맞는데, 예를 들어 private 함수 2개가 있을 때, 이 두 함수 간에 공백 라인이 없어서 보기에 약간 불편하다는 의미였습니다! 코드 자동 정렬해주면 자동으로 띄워주더라구요!

여기서 모드 구분을 위해 enum class 사용하면 더 좋을 거 같아요! SearchActivity에 달았던 리뷰 참고해주세용

tvGuide 보다 조금 더 구체적으로 id를 작성해주면 좋을 거 같습니다 :)

맞습니다.. 신경 안 쓰고 너무 막 지었어 딱 걸렸네요. (베였다)

!! (non-null assertion) 연산자를 사용하지 않고 구현할 수 있는 방법은 없을까요?!

이 부분 안 그래도 얘기를 한 번 하려고 했습니다!

그동안 작업할 때 편의상 !!를 써왔었는데 토스에서 근무중이신 개발자분한테 여쭤봤었거든요. !!는 거의 안 쓴다고 하시더라구요. 하긴 안정적인 설계를 위해선 번거롭더라도 null 처리를 해주는 게 맞긴 했습니다.

지금 기존의 코드 상당 부분에 !!가 쓰여있는데 이거에 대한 null 처리를 어떻게 해줄지 간단하게 컨벤션 정해보고 보일 때마다 틈틈이 고칠 계획입니다.

보여주신 코드에서는 searchResult가 SearchResultEntity 타입인데 이게 지금 non-nullable 타입입니다. 이걸 nullable하게 바꿔주고 (ex. private lateinit var searchResult: SearchResultEntity?)

searchResult?.locationLatLng?.longitude ?: 0.0 이런 식으로 해볼 수 있을 것 같은데 이렇게 하면 아까 남겨주셨던 리뷰와 비슷한 맥락으로 searchResult부터 null이 뜰 때, searchResult는 null이 아니지만 그 안에 있는 locationLatLng이 null일 때 모두 0.0이 세팅이 되는 거라 side effect가 생길 수 있을 것 같아 썩 안정적이라는 생각은 안 듭니다. (둘 중 어떤 이유로 0.0이 뜬 건지 구분이 바로 안 되기 때문)

저도 null 처리에 대한 경험이 많지 않아 옳은 방법인지는 모르겠으나

현재 짜놓은 코드 기준으로 제가 봤을 땐 searchResult가 null이 뜰 일이 없어보이는데 이걸 기본 전제로 깔고 간다면
searchResult는 null이 아니지만 그 안에 있는 건 nullable 할 수 있으니 (ex. locationLatLng) 이거에 대한 null check을 해야 할 거고

그렇다면

        val locationLatLng = searchResult.locationLatLng
        if (locationLatLng != null) {
            setDepartureLatLng(
                latLng = LatLng(
                    locationLatLng.latitude, locationLatLng.longitude
                )
            )
        } else { //로그를 찍거나 임의의 값 할당}

이렇게 해볼 수 있을 것 같네요.

그런데 이 경우 Log만 찍는다면 departureLatLng이 초기화가 안 되기 때문에 NPE가 떠서 앱이 죽을 거라
차라리 임의의 값을 할당하고 Log 또는 toast message로 null이 떴다는 걸 가시적으로 알리는 게 나을 것 같습니다.

searchResult는 어떤 이유에서든 null이 되면 안 되는 게 이 값이 null이 돼버리면 DrawActivity를 아예 돌릴 수가 없어서 정 통제를 해야겠다 싶으면 SearchActivity에서 intent에 정상적으로 값 할당이 됐는지 확인하고 안 그런 경우엔 아예 startActivity가 안 돌아서 DrawActivity로 못 넘어오게 하면 될 것 같습니다.

그런데 이렇게까지 해야 하나 하는 생각이 들긴 합니다. (잘 모르지만 개인적인 소감)

departureLatLng가 널이 되는 것을 방지하기 위해 임의의 값을 할당한다고 하셨는데, 어떤 값을 넣을 수 있을까요??

아무 좌표나 넣어도 상관은 없습니다!

확인 완료했습니다! 동기 처리 대신에 사용할 수 있는 방법이군요!